Step3 #5723
Step3 #5723
Conversation
There was a problem hiding this comment.
슬비님, PR이후 미리 작업을 해두고싶다면 shelve혹은 stash를 통해 상태를 저장 후 pr이 머지된 경우 플로우에 따라 rebase 이후 읽어와서 합쳐주시면 됩니다. rebase하기전 commit하거나 push하는 변경사항 반영만 안해주시면 됩니다.
그리고 질문 주신부분들에 대해서는 코멘트에 남겨드렸으니 같이 확인해보시면 좋을 것 같아요.
현재 객체 테스트가 힘든 이유는 객체에서 비즈니스 로직과 입/출력이 공존하고 절차지향적으로 하나의 함수에서 너무 많은 책임을 수행하고 있기 때문에 테스트가 힘들것으로 보여요 😢
좀 더 적절히 함수를 나눠보고 제가 참고하라고 링크드린 문서들을 한 번 읽어보신 뒤 고민해보시면 도움이 될 거라고 봅니다.
그럼 리뷰확인 후 다시 요청 부탁드려요!
| import java.util.InputMismatchException; | ||
| import java.util.Scanner; | ||
|
|
||
| public class InputView { |
There was a problem hiding this comment.
해당 클래스를 보면 별도의 상태를 가지지 않는 클래스로 보여요. 유틸리티 혹은 싱글톤 패턴을 적용해보면 어떨까요?
There was a problem hiding this comment.
@catsbi 코멘트 주신 부분이 이해가 안되서 다시 여쭤봅니다..!
별도의 상태를 가지지 않는 클래스란 말이 활용이 안되고 있는 클래스란 말이신걸까요?
There was a problem hiding this comment.
좀 더 직접적으로 설명을 드리면,
상태를 가지지 않고 있다는 말은 별도의 인스턴스 변수를 가지지 않는 클래스다. 라고 생각하시면 됩니다!
이런 객체들은 보통 재사용성이 높은 객체들인데 매 번 새롭게 인스턴스를 생성해서 사용할 필요가 없죠.
그렇기에 싱글톤 패턴을 적용해 재사용성을 높힐 수 있습니다.
|
|
||
| public static int racingQuestion(String msg) { | ||
| int carCount = 0; | ||
| while (true) { |
There was a problem hiding this comment.
특정한 중단포인트가 없는 무한루프는 권장드리지 않습니다. StackOverFlow 에러를 발생기키기 쉬운 코드이기도 하구요.
특정 중단 변수를 지정하거나 하는게 나을 것 같습니다.
또한 해당 반복문은 매 번 Scanner를 새로 호출하고 있는데, 그럴 필요가 있는 작업일까요? 재사용성이 없는 객체가 아닌걸로 보입니다!
| if(carCount <= 0) { | ||
| throw new InputMismatchException(); | ||
| } |
There was a problem hiding this comment.
이 유효성 검증이 view layer에서 직접 체크해서 재시도하기엔 비즈니스적인 로직으로 보이는데, 여기서 책임지는게 맞을까요 🤔
| public static int racingQuestion(String msg) { | ||
| int carCount = 0; | ||
| while (true) { | ||
| try { |
There was a problem hiding this comment.
code 의 indent가 3을 넘어가는 것 같을때는 책임이 적절한지 확인 후 메서드 분리를 고려해보세요!
|
|
||
| public class RacingCar { | ||
|
|
||
| public static final String FORWARD = "-"; |
| public static void racing() { | ||
|
|
||
| int tryCount = InputView.racingQuestion(RacingMessage.TRY_COUNT.msg()); | ||
| int carCount = InputView.racingQuestion(RacingMessage.CAR_COUNT.msg()); | ||
|
|
||
| for(int i= 0; i < tryCount; i++) { | ||
| for(int j = 0; j < carCount; j++) { | ||
| int count =0; | ||
| int randomNumber = NumberUtils.randomNumberUnder10(); | ||
| boolean isForward = randomNumber >= 4; | ||
| if(isForward) { | ||
| count++; | ||
| } | ||
| goAndStop(j, count); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 함수가 Controller, Service, Domain까지 모든 계층을 책임지는 절차지향 코드로 보입니다. 이렇게 될 경우 테스트 케이스를 작성하지 힘들 것 같네요. 실제로 테스트 케이스도 작성되지 못한 것 같아요!
우선 MVC 패턴과 SOLID 원칙에 대해 알아보면 좋을 것 같은데 관련 문서는 다음 포스팅을 참고해주세요
(MVC Pattern이란) (SOLID 원칙이란)
이 부분을 다 꼭 읽어보시되 좀 더 요약해서 전달드리면,
- View: 입/출력만 책임진다.
- Controller: View의 요청을 Service로 전달 후 응답을 View로 반환하여 중개자 역할을 책임진다.
- Service: 각 도메인간의 책임들을 적절히 오케스트레이션하여 전체적인 비즈니스 플로우를 완성한다.
- Domain: 각각이 담당한 Actor 책임을 수행하여 상태를가지고 상태를 적절히 관리한다.
그리고 해당 미션의 요구사항을 보시면 들여쓰기의 수준이 2를 넘지 않도록 요구하고 있으니 참고해주세요!
이번 과정을 통해 연습할 원칙은 다음 두 가지이다.
규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만 한다.
규칙 2: else 예약어를 쓰지 않는다.
이 두가지 원칙을 통해 메소드를 분리해 메소드가 한 가지 작업만 담당하도록 구현하는 연습을 목표로 한다.
이 같은 원칙 아래에서 메소드의 라인 수를 15라인이 넘지 않도록 구현한다.
리뷰 요청드립니다.
진행하면서 고민이 되었던 부분이 리뷰 해주시는 동안 다음 단계를 먼저 진행하고싶을때
브랜치를 먼저 새로 생성하고 그 브랜치에서 하면되는지 궁금합니다.
아니면 기존에 했던 브랜치에서 하다가 stash 하고 다시 수정사항을 커밋하면 되는지요ㅠㅠ?
또 이번 미션을 진행하면서 어려웠던 부분이 결과 화면을 출력하는 부분을 따로 나눠서 했어야했는지 궁금합니다.
왜냐면 화면 출력을 하려면 또 for문을 돌려야될것같은데 그러면 같은 코드를 또 사용하는것 같아 클린하지 못한 느낌이 들어서입니다.
그래서 안에 method를 분리해 사용하였는데 어떤 식으로 하면 좋을지 코멘트 부탁드립니다.
그리고 이번 테스트 코드는 작성을 어떻게 해야되는지 감이 안잡혀서 호출해서 에러가 날 부분을 미리 try-catch로 묶고 정리했는데 어떤식으로 테스트 코드 작성하면 좋을지도 궁금합니다...!
감사합니다.